Skip to content

feat: existing permission banner on permission picker#286

Closed
mj-kiwi wants to merge 9 commits intomainfrom
feat/existing-permission-banner
Closed

feat: existing permission banner on permission picker#286
mj-kiwi wants to merge 9 commits intomainfrom
feat/existing-permission-banner

Conversation

@mj-kiwi
Copy link
Contributor

@mj-kiwi mj-kiwi commented Mar 19, 2026

Description

Implement an existing-permissions banner in the permission picker to surface any permissions the user has already granted to a site for the requested chains/accounts.

This update depends on changes in the extension: MetaMask/metamask-extension#40995

Key Changes:

  • Display existing permissions in a banner/section above the new permission request details

Related issues

Fixes:

Manual testing steps

Screenshots/Recordings

Before

After

Screen.Recording.2026-03-19.at.2.17.52.PM.mov

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Changes the permission-request flow UI/logic by removing the blocking existing-permissions dialog and replacing it with a new banner driven by background permission lookups; this impacts core confirmation rendering and could regress gating/UX if the new status logic or wiring is incorrect.

Overview
Adds an existing-permissions banner to the permission confirmation screen that links out to a MetaMask “gator permissions” page, showing a warning when the user previously granted a similar permission type (stream vs periodic) and an info banner when prior permissions exist but are dissimilar.

Removes the entire core/existingpermissions dialog flow (content rendering, token-metadata formatting, UI components, and tests) and replaces it with a new ExistingPermissionsService.getExistingPermissionsStatus() in services/ that computes a single ExistingPermissionsState used during confirmation rendering.

Updates the permission lifecycle/orchestrator and handler types to fetch this status in parallel with the intro flow (with a non-rejecting promise) and pass it through to PermissionHandlerContent; updates i18n strings accordingly and adjusts snapshot tests for the new banner output.

Written by Cursor Bugbot for commit d44f14f. This will update automatically on new commits. Configure here.

@mj-kiwi mj-kiwi marked this pull request as ready for review March 19, 2026 01:24
@mj-kiwi mj-kiwi requested a review from a team as a code owner March 19, 2026 01:24
Copy link
Contributor

@jeffsmale90 jeffsmale90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks super good!

It's definitely less imposing than the full screen display.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

@mj-kiwi mj-kiwi requested a review from jeffsmale90 March 19, 2026 08:21
Copy link
Contributor

@jeffsmale90 jeffsmale90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great! A huge improvement over the full screen.

},
"existingPermissionsSimilarDescription": {
"message": "You have granted similar permissions to this site in the past. Review them"
"existingPermissionsSimilarMessage": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still requires that the link is the final part of the message, which will be problematic for some languages.

I think we can skip over this, but it might come back to bit us later.

scanAddressResult: FetchAddressScanResult | null;
hasExistingPermissions: boolean;
similarPermissionsExist: boolean;
existingPermissionsStatus: ExistingPermissionsState;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the distinction between "State" and "Status" here is confusing.

@mj-kiwi mj-kiwi marked this pull request as draft March 20, 2026 04:38
@mj-kiwi
Copy link
Contributor Author

mj-kiwi commented Mar 22, 2026

Due to our decision to use a different approach to display existing permissions #288, we are going to close this PR.

@mj-kiwi mj-kiwi closed this Mar 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants